savePersistedViewState reads the global viewStates dictionary from contextProxy, mutates states[this.viewStateId] in memory, and writes it back asynchronously via await contextProxy.setValue("viewStates", ...). When concurrent webview instances update mode or API profile selections simultaneously, one instance reads stale global state before the other's write completes, causing a lost update on viewStates.

resetState() clears global settings in contextProxy but does not clear this.viewLocalState (e.g., via _clearViewLocalState()). When getState(viewStateId) runs, it merges stale viewLocalState overrides over the reset contextProxy defaults, causing pre-reset or deleted profile settings to persist in active webview instances.

_updateViewLocalStateFromMutation updates in-memory viewLocalState in response to setValue/setValues calls, but never invokes savePersistedViewState. Mutations made via setValue/setValues are held only in memory and lost when the webview reloads or VS Code restarts.

The test 'should isolate mode state between instances' reads the initial mode of two provider instances without mutating mode in either, making it incapable of verifying whether mode changes in one instance leak to other instances.

The webviewDidLaunch handler test passes viewStateId: 'view-1' but omits an assertion verifying that provider.setViewStateId was called with 'view-1'.

Does this actually prove isolation? provider2 starts at "code" (the global default) and is never mutated, so this assertion passes whether or not the two instances share state. Would it be more meaningful to first set provider2 to a distinct non-default mode (e.g. "debugger"), then mutate provider1, then assert provider2 still reads "debugger"?

Already addressed — the isolation test now sets the second provider to a distinct non-default mode before mutating the first provider. Specifically, provider2 is set to debugger, then provider1 is set to architect. The assertions then verify provider1 reads architect while provider2 still reads debugger.

So the test now proves instance isolation with two distinct non-default/local states instead of relying on the global default.

The test confirms the new bedrock keys are present, but the PROVIDER_SETTINGS_KEYS merge at line 3118 of ClineProvider.ts spreads onto the existing viewLocalState.apiConfiguration — so stale keys from the previous provider (e.g. openRouterModelId) could survive. Worth adding expect((provider as any).viewLocalState.apiConfiguration).not.toHaveProperty("openRouterModelId") here?

Fixed — added the stale-key assertion on line 1068, so the test now verifies that switching from OpenRouter to Bedrock removes the previous OpenRouter model key. The implementation already handles this on line 3143: when a flat provider-settings update includes the provider field, it replaces the local API configuration instead of merging into the existing one, so stale provider-specific keys cannot survive. Verified with the parallel-mode spec: 42 tests passed.

crypto.randomUUID is mocked to always return "memory-view", so calling getViewStateId() twice will return the same string regardless of whether the early-return guard (that reads the stored ID) is present or not. What if the mock returned a different value on the second call — would the second getViewStateId() still return "memory-view"?

Fixed — the UUID mock now returns different values on subsequent calls (line 64). The assertions on lines 83-85 verify that the second call still returns "memory-view" and that UUID generation only happens once, so the test now proves the stored/in-memory ID is reused rather than regenerated. Verified: 3 tests passed.

The production code replaces non-alphanumeric/dash characters with underscores: .replace(/[^A-Za-z0-9_-]/g, "_"). All calls here use already-clean IDs — is there a test somewhere that passes a raw ID containing spaces, dots, or slashes to verify the sanitization actually runs and that the sanitized form is the key used in viewStates?

Yes — there is coverage for this now. The test at ClineProvider.parallelMode.spec.ts passes a raw view state ID containing a slash, dot, and spaces: tab panel/with.dots and spaces. It then verifies that the persisted viewStates key is the sanitized form on ClineProvider.parallelMode.spec.ts, and also verifies the raw unsanitized key is not present on ClineProvider.parallelMode.spec.ts. The parallel-mode spec was verified passing with 42 tests.

this.viewStateId is read inside the .then() closure at execution time. If setViewStateId() fires between when this write is enqueued and when the queue runs it, the write would land under the new ID rather than the one that was active when the change was made. Should viewStateId be captured before the .then() (e.g. const viewStateId = this.viewStateId at the top of savePersistedViewState)?

Yes — fixed. savePersistedViewState now captures the active viewStateId before the queued write is created on line 454, and the queued closure uses that captured ID on lines 457-458 instead of reading this.viewStateId later: ClineProvider.ts.

There is also regression coverage starting on line 1115: ClineProvider.parallelMode.spec.ts. The test starts a save under view-a on line 1139, switches to view-b before the queued write completes on line 1141, then verifies the write lands under view-a on lines 1145-1148: ClineProvider.parallelMode.spec.ts.

viewLocalState is updated here before savePersistedViewState is awaited. If the durable write fails, the in-memory state is ahead of what was persisted — on next reload viewLocalState would be re-hydrated from the old globalState value. Is that acceptable, or should the in-memory update only happen after the persist succeeds?

Good catch — I changed the ordering so the in-memory viewLocalState update only happens after durable persistence succeeds. The shared helper now awaits persistence first on line 3091, then updates in-memory state on line 3092: ClineProvider.ts.

I also added regression coverage starting on line 847: ClineProvider.parallelMode.spec.ts. The test forces the persisted viewStates write to fail on line 854, verifies saveViewState(...) rejects on line 858, and confirms viewLocalState was not advanced on line 859: ClineProvider.parallelMode.spec.ts.

saveViewState("currentApiConfigName", name) at line 1823 already writes both keys into viewLocalState (line 561). Does _updateViewLocalStateFromMutation here need to repeat that write, or is it a no-op? The setValue/setValues path only calls _updateViewLocalStateFromMutation + _persistViewLocalStateFromMutation without the preceding saveViewState — seems like these two paths could be consolidated.

Fixed. I consolidated the view-local state update/persist flow so profile activation no longer calls saveViewState(...) and then repeats the same local-state writes through the mutation sync path. Both activateProviderProfile and activated upsertProviderProfile now use the shared mutation helper directly, while setValue / setValues and saveViewState also route through the same helper.

I also added assertions that profile activation does not call saveViewState(...), which catches the duplicate path if it comes back.

clearPersistedViewState is defined here but I cannot find any call site for it. When a tab-panel ClineProvider is disposed, its viewStates entry is never cleaned up — it lives until the 50-entry prune evicts it by age. Should this be called from dispose()?

Yes — fixed. dispose() now clears persisted view-local state for editor/tab providers on line 891: ClineProvider.ts. It is guarded to editor render contexts starting on line 889, so sidebar providers keep their stable persisted state: ClineProvider.ts.

I also added test coverage starting on line 1153: ClineProvider.parallelMode.spec.ts. The test seeds a viewStates entry, disposes the editor provider on line 1160, and verifies the persisted key is removed on line 1162: ClineProvider.parallelMode.spec.ts.